Add bulk finalization #5734

seadowg · 2023-09-11T18:18:49Z

This makes it possible to finalise all forms in "Drafts" without needing to open any of them. There's still a bunch of different cases and kinds of forms we need to handle, and that is all listed in "Follow on work" in the issue (I'll write these up as their own issues).

I've also made a change to our linting here so that ./gradlew checkCode does not check for hardcoded strings, but that CI does using a different lint command (lintDebug -PlintStrings). This makes it easier to work on a PR without translated strings and then add them at the end.

As this PR adds some strings, we'll need to add them to master after we merge. The strings to add are:

finalize_all_forms
bulk_finalize_success
bulk_finalize_failure
bulk_finalize_partial_success
incomplete

What has been done to verify that this works as intended?

New and existing tests.

Why is this the best possible solution? Were any other approaches considered?

I mainly follow the approach we're now using for forms of having a data service that accesses repos and then exposing UI state to the Activity/Fragment with a ViewModel. One thing to note is that so that this will be easier to merge in later, I've made as few changes to existing code so there are definitely missed opportunities to share the new code with the existing form loading/saving flows.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

As I said above, I've made as few changes to existing code which should hopefully also lower the risk here. In terms of testing the new functionality: it'd definitely be good to check bulk finalization with as many different kinds of forms as you can think of! The kinds of scenarios that I expect not work are listed in "Follow on work" in the issue, but still feel free to try these out if only to confirm they don't actually work.

Before submitting this PR, please make sure you have:

run ./gradlew checkAll and confirmed all checks still pass OR confirm CircleCI build passes and run ./gradlew connectedDebugAndroidTest locally.
verified that any code or assets from external sources are properly credited in comments and/or in the about file.
verified that any new UI elements use theme colors. UI Components Style guidelines

We're not adding more predicate caching in this release, and this setting makes FormEntryController creation dependent on the current project.

seadowg · 2023-09-27T15:22:18Z

@alyblenkin @lognaturel just to clarify: should we mark a form that hasn't need validated yet as "incomplete" or "complete"? I was assuming "incomplete", but wanted to double check.

alyblenkin · 2023-09-27T16:15:19Z

@alyblenkin @lognaturel just to clarify: should we mark a form that hasn't need validated yet as "incomplete" or "complete"? I was assuming "incomplete", but wanted to double check.

Yes, that's what I was thinking. The goal is to give the user error feedback earlier.

Incomplete = saved with errors or we don't know
Complete = saved with no errors

And then we need to change the validation button at the end from "check for errors" to "check for completeness" once we decide that this is the language we want to move forward with.

seadowg · 2023-09-27T16:31:33Z

And then we need to change the validation button at the end from "check for errors" to "check for completeness" once we decide that this is the language we want to move forward with.

Good point. Just to be clear, for this PR you'll never actually see "Complete" as there will be no way to see a form that's saved and valid in Drafts. That will come with #5718 (which I'll update for the new design).

alyblenkin · 2023-09-27T16:34:25Z

And then we need to change the validation button at the end from "check for errors" to "check for completeness" once we decide that this is the language we want to move forward with.

Good point. Just to be clear, for this PR you'll never actually see "Complete" as there will be no way to see a form that's saved and valid in Drafts. That will come with #5718 (which I'll update for the new design).

Yes exactly! Sorry I was writing it out more for my own clarity. @lognaturel let me know if you are thinking about it differently.

lognaturel · 2023-09-27T19:37:45Z

That all sounds great to me!

"Incomplete" feels like a good choice for "we don't know" because it forces the user to open that submission again and take an explicit action. The worst case with that would be that they get stuck in a loop opening the submission, deciding everything looks good, tapping the back button to exit, again seeing "Incomplete", and repeating. Hopefully at some point they'd try the finalize/send button if their intent is to send.

If complete and "we don't know" submissions look the same, then users could assume that a particular draft is done for now and fail to capture important information. That feels worse than the possible annoying loop described above.

seadowg · 2023-09-28T13:27:07Z

@grzesiek2010 @lognaturel this is ready for another look. The pill/chip doesn't look quite right yet, but @alyblenkin and I discussed this and seeing as a "non-clickable" chip (something that we and it seems the Material 3 website CSS refer to as a "pill") doesn't official exist, we're going to pair on building this out properly a little later.

grzesiek2010 · 2023-09-29T09:31:23Z

collect_app/src/main/java/org/odk/collect/android/instancemanagement/InstanceListItemView.kt

+import java.util.Date
+import java.util.Locale
+
+object InstanceListItemView {


Why this is not a custom view but a class that only sets its state?. This is not something we used to do.

I should have left a comment about this. The problem is that it's used by a SimpleCursorAdapter which does the view inflation itself - I'm trying to make as little changes to existing code as possible to make merging easier later. I didn't want to have to refactor out the cursor, so this was where I ended up. I guess maybe a better approach would be to make this a ViewHolder that takes a view in the constructor. That would be overkill for now, but would plugin straight to a RecyclerView when all this gets refactored. What do you think?

I think that adding a comment in the class that at some point it should be a custom view as the class name says is ok for now.

grzesiek2010 · 2023-09-29T09:33:31Z

collect_app/src/main/java/org/odk/collect/android/instancemanagement/InstanceListItemView.kt

+        setUpSubtext(view, instance, context)
+
+        val chip = view.findViewById<TextView>(R.id.chip)
+        if (chip != null) {


Is it possible that it's null?

Yeah because the adapter sometimes uses a different layout (form_chooser_list_item_multiple_choice.xml).

grzesiek2010 · 2023-09-29T09:37:13Z

collect_app/src/main/java/org/odk/collect/android/instancemanagement/InstanceListItemView.kt

+        disabledCause.text = disabledMessage
+
+        // Material design "disabled" opacity is 38%.
+        formTitle.alpha = 0.38f


I think it would be good to factor out this constant.

I think this might not even be correct anymore. I'll pull it out as a constant for this file, but don't want to get distracted by restyling the whole list item right now.

seadowg · 2023-09-29T13:32:33Z

@grzesiek2010 @lognaturel I think we should skip QA for this PR as it leaves things in a confusing state (all drafts end up incomplete) and then test everything as part of the PR for #5718.

lognaturel · 2023-09-29T18:48:37Z

I agree with merging this and then doing QA when #5718 is in.

I'm seeing the literal string "Success! %d forms finalized". Maybe that's something you intend to come back to once we finalize all language but wanted to flag in case it's unexpected.

We'll also need to trigger autosend after bulk finalization completes. That could be tracked and addressed separately.

Looks good to me beyond that!

seadowg · 2023-10-02T09:08:52Z

I'm seeing the literal string "Success! %d forms finalized". Maybe that's something you intend to come back to once we finalize all language but wanted to flag in case it's unexpected.

Fixed!

Add strings from PR #5734

Add bulk finalization

seadowg changed the base branch from master to v2023.3.x September 11, 2023 18:19

seadowg force-pushed the bulk-finalization branch from dc2d6c9 to 6e78da6 Compare September 12, 2023 11:48

seadowg added 21 commits September 13, 2023 10:44

Spike out use cases for form entry loading and init

932362f

Spike out use case for finalizing a draft

1f7917a

Improve interface for use with existing loading/saving code

a229383

Allow all instances to be finalized in one click

104fc93

Use InstancesAppState to update drafts list

764a185

Only check for hardcoded text on CI

90865b7

Rename InstancesAppState to be inline with FormsDataService

fc52e7f

Use AppState in InstancesDataService

9ef6db9

Move bulk finalization logic to data service

3abf938

Remove option to disable predicate caching

1f61d24

We're not adding more predicate caching in this release, and this setting makes FormEntryController creation dependent on the current project.

Construct FormEntryController factory in data service

d5b33d3

Allow EntitiesRepository to be provided for the current project

3836354

Remove CurrentProjectProvider dependenccy from InstancesDataService

96fc2e1

Remove Context dependency from InstancesDataService

279bc9e

Only finalize drafts

b8a3003

Show progress dialog when finalizing forms

599d87b

Show different snackbar when form finalization fails

e95aaa3

Show instance in different state when validation has failed

58e8f09

Allow editing invalid drafts

19d5479

Don't revalidate invalid forms

6721f7a

Add test for partial submissions

9d45a71

seadowg force-pushed the bulk-finalization branch from 48efb63 to 9d45a71 Compare September 13, 2023 10:23

seadowg added 6 commits September 13, 2023 13:03

Use real FormController to test FormEntryUseCases

24a9b26

Add operations that could be used in real code to FormEntryUseCases

03579b9

Handle partial forms in finalizeDraft

dd5bf1c

Rename method

ee2af36

Media dir shouldn't be shared

459b1f5

Allow reference manager to be setup without a StoragePathProvider

89f217f

Add basic styling to chip

e17609e

seadowg added 2 commits September 27, 2023 16:26

Show incomplete chip for incomplete drafts

545c82d

Add icon to incomplete chip

e35d2f5

Add string for incomplete

415e8b7

seadowg requested a review from lognaturel September 28, 2023 13:25

grzesiek2010 reviewed Sep 29, 2023

View reviewed changes

Pull out constant for disabled value

df74a3f

seadowg requested a review from grzesiek2010 September 29, 2023 11:05

Add deprecation notice to method that should be replaced

cdd3745

grzesiek2010 approved these changes Sep 30, 2023

View reviewed changes

Fix quantity strings

6dc15fd

seadowg merged commit 54966e9 into getodk:v2023.3.x Oct 2, 2023

seadowg deleted the bulk-finalization branch October 2, 2023 09:36

seadowg added a commit to seadowg/collect that referenced this pull request Oct 2, 2023

Add strings from PR getodk#5734

dce56af

grzesiek2010 added a commit that referenced this pull request Oct 2, 2023

Merge pull request #5757 from seadowg/bulk-finalization-strings

aa6794a

Add strings from PR #5734

seadowg mentioned this pull request Oct 3, 2023

Validate drafts when saved from end screen #5760

Merged

3 tasks

seadowg added a commit to seadowg/collect that referenced this pull request Oct 17, 2023

Merge pull request getodk#5734 from seadowg/bulk-finalization

c49c8f7

Add bulk finalization

seadowg added a commit to seadowg/collect that referenced this pull request Oct 24, 2023

Merge pull request getodk#5734 from seadowg/bulk-finalization

49ebda1

Add bulk finalization

seadowg added a commit to seadowg/collect that referenced this pull request Oct 30, 2023

Merge pull request getodk#5734 from seadowg/bulk-finalization

056b29e

Add bulk finalization

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add bulk finalization #5734

Add bulk finalization #5734

seadowg commented Sep 11, 2023 •

edited

Loading

seadowg commented Sep 27, 2023

alyblenkin commented Sep 27, 2023

seadowg commented Sep 27, 2023

alyblenkin commented Sep 27, 2023

lognaturel commented Sep 27, 2023

seadowg commented Sep 28, 2023

grzesiek2010 Sep 29, 2023

seadowg Sep 29, 2023 •

edited

Loading

grzesiek2010 Sep 29, 2023

seadowg Sep 29, 2023

grzesiek2010 Sep 29, 2023

seadowg Sep 29, 2023

grzesiek2010 Sep 29, 2023

seadowg Sep 29, 2023

seadowg commented Sep 29, 2023

lognaturel commented Sep 29, 2023 •

edited

Loading

seadowg commented Oct 2, 2023

Add bulk finalization #5734

Add bulk finalization #5734

Conversation

seadowg commented Sep 11, 2023 • edited Loading

What has been done to verify that this works as intended?

Why is this the best possible solution? Were any other approaches considered?

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Before submitting this PR, please make sure you have:

seadowg commented Sep 27, 2023

alyblenkin commented Sep 27, 2023

seadowg commented Sep 27, 2023

alyblenkin commented Sep 27, 2023

lognaturel commented Sep 27, 2023

seadowg commented Sep 28, 2023

grzesiek2010 Sep 29, 2023

Choose a reason for hiding this comment

seadowg Sep 29, 2023 • edited Loading

Choose a reason for hiding this comment

grzesiek2010 Sep 29, 2023

Choose a reason for hiding this comment

seadowg Sep 29, 2023

Choose a reason for hiding this comment

grzesiek2010 Sep 29, 2023

Choose a reason for hiding this comment

seadowg Sep 29, 2023

Choose a reason for hiding this comment

grzesiek2010 Sep 29, 2023

Choose a reason for hiding this comment

seadowg Sep 29, 2023

Choose a reason for hiding this comment

seadowg commented Sep 29, 2023

lognaturel commented Sep 29, 2023 • edited Loading

seadowg commented Oct 2, 2023

seadowg commented Sep 11, 2023 •

edited

Loading

seadowg Sep 29, 2023 •

edited

Loading

lognaturel commented Sep 29, 2023 •

edited

Loading